Guilt/check notification json #15

daywalker90 · 2025-08-13T16:14:38Z

Had to remove this line

typename = e.typename

in

contrib/msggen/msggen/gen/rpc/notification.py

and rerun msggen.

Particularly important since we're going to update the format: this makes sure we don't break them! Signed-off-by: Rusty Russell <[email protected]>

… in channel_state_changed notification We always prefer to omit fields rather than use 'null' (or unknown!). Note that before this, the schema was broken, so we have to put a special exemption in for that case. Signed-off-by: Rusty Russell <[email protected]>

Rather than forcing them to wrap their parameters in a "payload" sub-object, copy in params directly. We include the "origin" field one level up, if they care. The next patch restores compatibility for the one place we currently use them, which is the pay plugin. Signed-off-by: Rusty Russell <[email protected]>

…ame. All the core notifications changed over to wrapping the notification fields in an object with the name of the notification, but notifications from plugins were missed. Changelog-Added: Plugins: `channel_hint_update`, `pay_failure` and `pay_success` notifications now have objects of the same name containing the expected fields. Changelog-Deprecated: Plugins: `channel_hint_update`, `pay_failure` and `pay_success` notification fields outside the same-named object. Signed-off-by: Rusty Russell <[email protected]>

…ugins. Signed-off-by: Rusty Russell <[email protected]>

…ion schemas. Note that we need a workaround for deprecated APIs where "channel_state_changed" output "null" which violated the schema. Signed-off-by: Rusty Russell <[email protected]>

…ications. Modern style for notifications is to put everything inside an object of same name as the method. For now this means duplication for backward compatibility. ChatGPT helped me do that. Signed-off-by: Rusty Russell <[email protected]>

Changelog-None

…t.json This is done by tests/test_connection.py::test_websocket: ``` { "jsonrpc": "2.0", "method": "connect", "params": { "connect": { "id": "031b84c5567b126440995d3ed5aaba0565d71e1834604819ff9c17f5e9d5dd078f", "direction": "in", "address": { "type": "websocket", "subtype": "ipv4", "address": "127.0.0.1", "port": 59412 } } } } ``` Signed-off-by: Rusty Russell <[email protected]>

daywalker90 · 2025-08-13T16:15:49Z

This should fix the new error in CI:

error[E0412]: cannot find type `ConnectAddressType` in this scope
   --> cln-rpc/src/notifications.rs:130:20
    |
130 |     pub item_type: ConnectAddressType,
    |                    ^^^^^^^^^^^^^^^^^^ not found in this scope
    |
help: consider importing this enum
    |
6   + use crate::model::responses::ConnectAddressType;
    |

error[E0412]: cannot find type `ConnectDirection` in this scope
   --> cln-rpc/src/notifications.rs:136:20
    |
47  | pub enum PeerConnectDirection {
    | ----------------------------- similarly named enum `PeerConnectDirection` defined here
...
136 |     pub direction: ConnectDirection,
    |                    ^^^^^^^^^^^^^^^^
    |
help: an enum with a similar name exists
    |
136 |     pub direction: PeerConnectDirection,
    |                    ++++
help: consider importing this enum
    |
6   + use crate::model::responses::ConnectDirection;
    |

And add a check for new uses creeping in, since it got cut & paste everywhere. This means "this is a valid string, but truncate it to this many characters" vs "%.*s" which means "only read this many characters of string": ``` ['lightningd-3 2025-10-23T02:31:40.890Z **BROKEN** plugin-funder: Plugin marked as important, shutting down lightningd!'] --------------------------- Captured stderr teardown --------------------------- #0 0x557da58ad1dc in printf_common(void*, char const*, __va_list_tag*) asan_interceptors.cpp.o #1 0x557da5aff814 in json_out_addv /home/runner/work/lightning/lightning/ccan/ccan/json_out/json_out.c:239:11 #2 0x557da59740ce in plugin_logv /home/runner/work/lightning/lightning/plugins/libplugin.c:1777:2 #3 0x557da5969b6f in plugin_log /home/runner/work/lightning/lightning/plugins/libplugin.c:1934:2 #4 0x557da595c4f6 in datastore_del_success /home/runner/work/lightning/lightning/plugins/funder.c:161:2 #5 0x557da598b837 in handle_rpc_reply /home/runner/work/lightning/lightning/plugins/libplugin.c:1072:10 #6 0x557da598a4b0 in rpc_conn_read_response /home/runner/work/lightning/lightning/plugins/libplugin.c:1361:3 #7 0x557da5adbea5 in next_plan /home/runner/work/lightning/lightning/ccan/ccan/io/io.c:60:9 #8 0x557da5ae06ff in do_plan /home/runner/work/lightning/lightning/ccan/ccan/io/io.c:422:8 #9 0x557da5adfb58 in io_ready /home/runner/work/lightning/lightning/ccan/ccan/io/io.c:439:10 #10 0x557da5aec2ce in io_loop /home/runner/work/lightning/lightning/ccan/ccan/io/poll.c:455:5 #11 0x557da59757ac in plugin_main /home/runner/work/lightning/lightning/plugins/libplugin.c:2409:3 #12 0x557da594fe23 in main /home/runner/work/lightning/lightning/plugins/funder.c:1723:2 #13 0x7f6572229d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16 #14 0x7f6572229e3f in __libc_start_main csu/../csu/libc-start.c:392:3 #15 0x557da588b584 in _start (/home/runner/work/lightning/lightning/plugins/funder+0x10d584) (BuildId: 71ba63ab577fc6fa60573d3e8555f6db7d5c584d) 0x624000009d28 is located 0 bytes to the right of 7208-byte region [0x624000008100,0x624000009d28) allocated by thread T0 here: #0 0x557da590e7f6 in __interceptor_realloc (/home/runner/work/lightning/lightning/plugins/funder+0x1907f6) (BuildId: 71ba63ab577fc6fa60573d3e8555f6db7d5c584d) #1 0x557da5b2149b in tal_resize_ /home/runner/work/lightning/lightning/ccan/ccan/tal/tal.c:755:13 #2 0x557da59f2032 in membuf_tal_resize /home/runner/work/lightning/lightning/common/utils.c:203:2 #3 0x557da5b03934 in membuf_prepare_space_ /home/runner/work/lightning/lightning/ccan/ccan/membuf/membuf.c:45:12 #4 0x557da59d4289 in jsonrpc_io_read_ /home/runner/work/lightning/lightning/common/jsonrpc_io.c:127:2 #5 0x557da598a635 in rpc_conn_read_response /home/runner/work/lightning/lightning/plugins/libplugin.c:1366:9 #6 0x557da5adbea5 in next_plan /home/runner/work/lightning/lightning/ccan/ccan/io/io.c:60:9 #7 0x557da5ae06ff in do_plan /home/runner/work/lightning/lightning/ccan/ccan/io/io.c:422:8 #8 0x557da5adfb58 in io_ready /home/runner/work/lightning/lightning/ccan/ccan/io/io.c:439:10 #9 0x557da5aec2ce in io_loop /home/runner/work/lightning/lightning/ccan/ccan/io/poll.c:455:5 #10 0x557da59757ac in plugin_main /home/runner/work/lightning/lightning/plugins/libplugin.c:2409:3 #11 0x557da594fe23 in main /home/runner/work/lightning/lightning/plugins/funder.c:1723:2 #12 0x7f6572229d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16 SUMMARY: AddressSanitizer: heap-buffer-overflow asan_interceptors.cpp.o in printf_common(void*, char const*, __va_list_tag*) Shadow bytes around the buggy address: 0x0c487fff9350: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c487fff9360: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c487fff9370: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c487fff9380: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c487fff9390: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 =>0x0c487fff93a0: 00 00 00 00 00[fa]fa fa fa fa fa fa fa fa fa fa 0x0c487fff93b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c487fff93c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c487fff93d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c487fff93e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c487fff93f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb ==26122==ABORTING ``` Signed-off-by: Rusty Russell <[email protected]>

rustyrussell and others added 9 commits August 13, 2025 16:58

pytest: simple test for pay's custom notifications.

62f038b

Particularly important since we're going to update the format: this makes sure we don't break them! Signed-off-by: Rusty Russell <[email protected]>

lightningd: add dev option to save hooks and notifications to/from pl…

55b7413

…ugins. Signed-off-by: Rusty Russell <[email protected]>

pyln-testing: check plugin notifications against any extant notificat…

d869fa4

…ion schemas. Note that we need a workaround for deprecated APIs where "channel_state_changed" output "null" which violated the schema. Signed-off-by: Rusty Russell <[email protected]>

msggen: add override for connect notification in cln-rpc aswell

25260e2

Changelog-None

daywalker90 requested a review from cdecker as a code owner August 13, 2025 16:14

daywalker90 mentioned this pull request Aug 13, 2025

Actually check, cleanup, notification schemas ElementsProject/lightning#8376

Merged

rustyrussell force-pushed the guilt/check-notification-json branch 12 times, most recently from 55db5c8 to 1c5e7cf Compare August 17, 2025 00:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Guilt/check notification json #15

Guilt/check notification json #15

Uh oh!

daywalker90 commented Aug 13, 2025

Uh oh!

daywalker90 commented Aug 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Guilt/check notification json #15

Are you sure you want to change the base?

Guilt/check notification json #15

Uh oh!

Conversation

daywalker90 commented Aug 13, 2025

Uh oh!

daywalker90 commented Aug 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants